-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add preview workflow #279
base: main
Are you sure you want to change the base?
Add preview workflow #279
Conversation
.github/workflows/preview.yml
Outdated
# we run our artifact directly please use the prebuild | ||
# elastic/docs-builder@main GitHub Action for all other repositories! | ||
- name: Build documentation | ||
run: .artifacts/publish/docs-builder/release/docs-builder --strict |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we cache this from the pr.yml
build and then move the general docs build as part of this workflow ?
In other words we make this depend on the pr workflow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.github/workflows/preview.yml
Outdated
# elastic/docs-builder@main GitHub Action for all other repositories! | ||
- name: Build documentation | ||
run: .artifacts/publish/docs-builder/release/docs-builder --strict | ||
- name: Generate AWS Role ARN |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we also want to extend our publish
github action to include this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some general comments (realizing this is a draft).
9657bdb
to
c0a2f5b
Compare
.github/workflows/pr.yml
Outdated
- uses: actions/upload-artifact@v4 | ||
with: | ||
name: docs | ||
path: .artifacts/docs/html |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cache our binary instead of the html.
Our html is small but for e.g docs-content it will be many megabytes (e.g just the output of the converted obs docs is 100mb zipped due to all the images).
Secondly we can then either grab the binary or run our github action inside the preview action.
With the preview github action being the only one we expose to everyone to add to their PR builds.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense, thank you for the tip.
Yes, in the action for consumers it will be different. There won't be any uploading of artifacts .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.github/workflows/preview.yml
Outdated
- uses: actions/download-artifact@v4 | ||
with: | ||
name: docs | ||
path: .artifacts/docs/html |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here we should conditionally download the cached binary or run our github action to generate the docs.
name: docs | ||
path: .artifacts/docs/html | ||
|
||
- uses: ./.github/actions/aws-auth |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does this work when called from other elastic repositories?
I'd prefer this to be an action since we don't want to polute other UI's with all these steps (correct me if im wrong) but then auth might be an issue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I should have been clearer in my description.
This is implementation is not supposed to be used for the consumer, only for this repo.
I was planning to create a consumer action in a follow-up.
Ofcourse it will be highly inspired by this, and we will see if there can be any commons that we can refactor out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Few nits, can't wait to merge this in though so awesome!
deploy: | ||
runs-on: ubuntu-latest | ||
steps: | ||
- name: Create Deployment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not necessary for this PR but we might want to protect this to only run if doc changes are part of the PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My intention for this preview was not to create a preview of the docs only, but actually also a preview of changes within the docs-builder codebase.
Details
Add deployment of preview environments on a PR
Follow-ups